Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

events: Move implementation of events::send() to lib/events #499

Merged
merged 1 commit into from
Sep 21, 2023

Conversation

pussuw
Copy link

@pussuw pussuw commented Sep 21, 2023

Events have a global, system-wide sequence number, which must be handled atomically, (fetching and incrementing the sequence AND sending the event to uORB must be atomic). Currently in FLAT mode, only one instance of this sequence number exists, so it is OK to have it in px4_platform.

However, in PROTECTED mode px4_platform is instantiated both in kernel- and user spaces, which makes two instances of this sequence number, which causes problems in the mavlink event handling logic.

When mavlink receives and handles events, it expects that:

  • The sequence numbers arrive in order (seq n is followed by n+1 etc)
  • It increments by 1
  • There is only one instance of the sequence number

In PROTECTED mode this is violated, as the kernel and user sequence numbers run freely on their own. This patch fixes the issue by moving the event backend to the kernel and by providing user access to it via ioctl.

Events have a global, system-wide sequence number, which must be handled
atomically, (fetching and incrementing the sequence AND sending the event
to uORB must be atomic). Currently in FLAT mode, only one instance of this
sequence number exists, so it is OK to have it in px4_platform.

However, in PROTECTED mode px4_platform is instantiated both in kernel-
and user spaces, which makes two instances of this sequence number, which
causes problems in the mavlink event handling logic.

When mavlink receives and handles events, it expects that:
- The sequence numbers arrive in order (seq n is followed by n+1 etc)
- It increments by 1
- There is only one instance of the sequence number

In PROTECTED mode this is violated, as the kernel and user sequence
numbers run freely on their own. This patch fixes the issue by moving
the event backend to the kernel and by providing user access to it via
ioctl.
@pussuw pussuw requested a review from jlaitine September 21, 2023 10:35
Copy link

@jlaitine jlaitine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM try to send to upstream, as this fixes a bug in protected build as well

@pussuw
Copy link
Author

pussuw commented Sep 21, 2023

I'll merge this for now as it fixes a bug for us. The upstream solution might necessitate some modifications but this commit can just be dropped/replaced when rebasing.

@pussuw pussuw merged commit 2af6ce3 into main Sep 21, 2023
21 checks passed
@pussuw pussuw deleted the events_to_lib branch September 21, 2023 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants